Skip to content
This repository has been archived by the owner on Mar 26, 2024. It is now read-only.

(#16429) Relpaths #122

Merged
merged 4 commits into from Jan 3, 2013
Merged

(#16429) Relpaths #122

merged 4 commits into from Jan 3, 2013

Conversation

djmitche
Copy link
Contributor

Two of these fixes are simple changes to the CSS.

The third is a code change, but relatively minor.

I added a test for the code change, but I don't see an easy way to test that it gets the right path even at a sub-URI. Still, this shows that the code is executed and produces the correct result in at least one condition.

These links work when puppet-dashboard is at the top level because browsers treat /../ as equivalent to /.
With corresponding generated changes in application.css
This gets the path right even when the app is hosted at a sub-URI.
A basic test of the page looks for the link.
@djmitche
Copy link
Contributor Author

I just added a fourth for the radiator view, which I had missed initially.

@jeffweiss
Copy link
Contributor

@djmitche I need to verify that all the tests pass with these changes. In the interim, can you give me a bit more justification on the problem this is actually solving? Thanks!

@djmitche
Copy link
Contributor Author

They fix (partially) the case where you're running the dashboard at a base URL other than /. Most of the dashboard generates links using the Rails way, which handles this case smoothly. The fixes here address a few places where links were generated directly, and incorrectly for this case.

@sodabrew
Copy link
Owner

Ok I re-read the changes. In particular I thought you changed from ../ to ../../ -- but it was the other way around. I do wonder if there are any second-level-with-a-trailing-slash views that would be impacted.

Other than that, I'm comfortable with this PR.

@djmitche
Copy link
Contributor Author

Those that I changed, I observed doing the wrong thing in a running dashboard, so I tend to think not. But I can't prove it beyond this statement.

@sodabrew
Copy link
Owner

@puppetlabs Do you have any plugins that are extending the Dashboard routes to add third-level deep URL paths? If not, I'd like to merge this in.

@haus
Copy link
Contributor

haus commented Dec 20, 2012

@lifton @jeffweiss ^^

@djmitche
Copy link
Contributor Author

djmitche commented Jan 3, 2013

This will still work with third-level-deep URL paths. Paths in CSS are relative to the CSS file, not window.url.

All paths generated in Ruby use the controller to generate the path, so they'll be absolute and correct.

@sodabrew
Copy link
Owner

sodabrew commented Jan 3, 2013

@djmitche Oh. I'm a doofus. You're right, the images are relative to the css file's url, not to the current page url. Merging now!

sodabrew added a commit that referenced this pull request Jan 3, 2013
@sodabrew sodabrew merged commit 6aa0b98 into sodabrew:master Jan 3, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants